-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TL: bump version to 5.5.2 + added release guide for maintainers #489
Conversation
.github/workflows/ci_pipeline.yml
Outdated
@@ -4,6 +4,9 @@ name: CI pipeline for pySDC | |||
|
|||
on: | |||
push: | |||
branches: | |||
- '**' | |||
- '!new-release' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid running the full tests on a new-release
branch, since it will be done after once the PR is triggered
... saving a bit of energy 😉 🌍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the harm, though? You don't have to wait for the checks to pass before creating a PR. And if people dare to call their branch differently (see guide), this won't help. It's a special exception with no obvious/explained reason in the pipeline file I'd rather avoid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I was originally more in favor of "one name only" for the version bump branch ... and commit on new branches in fork appear to not be tested before the PR is triggered. It just felt a bit like a waste to test every commit we do when modifying the new release branch (e.g for this one, the 6 commits where tested while they shouldn't have)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The website is only built when pushing to master, though, no? So while you do test everything during the PR, the website is only updated to the new version number when the CI runs again after the merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm speaking about all the tests (projects, code, ...) that are run for all python version and for all commit that you do on this branch. Those are unnecessary since they will be run once the PR is triggered (when the branch is ready). At the end it wouldn't have change anything, but would have save a bit of energy consumption somewhere in a data-center by avoiding redundant tests.
But well, I chose my fights ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. Well you may still want to run them in case some versions have changed in a breaking way. And if not you can always just cancel manually. It's not like people are doing new releases every other hour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was my point : tests will be run anyway when the PR is triggered, so if the version change breaks the code, you'll see it. Hence you don't need to run those when pushing on this branch before ...
No description provided.